Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a CJS build #13

Closed
wants to merge 1 commit into from

Conversation

james-yeoman
Copy link

Addresses #12

@james-yeoman
Copy link
Author

@robinloeffel do you have any reservations against this PR? Without it, I'm unable to fully adopt v9, since floydspace/serverless-esbuild#498 prevents me from using ESM for my esbuild config

@robinloeffel
Copy link
Owner

Hey man! In the meantime, I went ahead and implemented your suggestions myself. With your proposed changes, the ESLint setup of the repo wouldn't have functioned anymore, since it requires a base tsconfig.json where it can derive some info for type-checking and such. Thank you for the effort you put in, I hope version 0.3.11 fixes your issues!

@james-yeoman
Copy link
Author

I really appreciate how responsive you've been so far Robin, and I hope I'm not coming across as "a bit much" with all the back-and-forth, issue creation, and so on. There's just one minor tweak that is needed for this particular problem to be fully resolved:

// package.json
{
  // ...
  "exports": {
    // ...
    "require": "./dist/cjs/index.cjs"
  },
  "main": "./dist/cjs/index.cjs",
  // ...
  "scripts": {
    // ...
    "build:cjs": "tsc --project tsconfig.cjs.json && mv dist/cjs/index.js dist/cjs/index.cjs"
    // ...
  }
  // ...
}

which resolves this error

Error [ERR_REQUIRE_ESM]: require() of ES Module
[my monorepo]/packages/api/node_modules/esbuild-plugin-eslint/dist/cjs/index.js
from [my monorepo]/packages/api/esbuild.config.cjs not supported.

index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains
"type": "module" which declares all .js files in that package scope as ES modules.

Instead either rename index.js to end in .cjs, change the requiring code to use dynamic import() which is
available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in
[my monorepo]/packages/api/node_modules/esbuild-plugin-eslint/package.json to treat all .js files as
CommonJS (using .mjs for all ES modules instead).

@james-yeoman james-yeoman deleted the exports-map branch March 13, 2024 09:31
@robinloeffel
Copy link
Owner

Don't worry about it, I'll just do it at my own pace! Should be all good now with the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants